Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

recalc apps vsn #2864

Closed
wants to merge 1 commit into from
Closed

recalc apps vsn #2864

wants to merge 1 commit into from

Conversation

nyczol
Copy link

@nyczol nyczol commented Feb 21, 2024

Use apps vsn that are generated during build process.

Eg. app vsn can be recalculated in post_hooks/compile

@ferd
Copy link
Collaborator

ferd commented Feb 21, 2024

The versions computed by compile should already be included -- there's even an app_compile hook point defined that deals with this, with a final step that validates the .app file's content. There's also a big preprocess block that deals with this and does extra-validation as well.

My guess is that if we want to modify the app file in hooks, that's where it needs to take place (specifically in the pre-hook if we want it properly tracked with validation; doing it in the post-hook goes after validation and isn't going to be safe so I don't think we carry the state then). There's other things in the version files and other providers that depend on them, so I'm not sure it's actually a good idea to go modify them arbitrarily in hooks after the fact. Other tasks may depend on release having been run (eg. the tar step does that) and it may not inherit the changes to the app file if they haven't been properly propagated to the ongoing state carried by rebar3 providers.

But before saying "no, this is a bad idea", I'd like to know what it is you're modifying here and why? Because this is possibly a valid use case, but it needs to be supported properly in a way that propagates the information adequately. Right now this patch fixes release's direct use, but does not carry the information downstream, and may only be a partial fix depending on the use case.

@nyczol
Copy link
Author

nyczol commented Feb 21, 2024

My use case is quite simple: app vsn is calculated after compilation by post_compile escript:
{post_hooks, [{compile, "./priv/post_compile"}, {release, "./priv/post_release"}]}.

post_compile escript uses modules hashes and dates to calculate application vsn, example below:
{vsn,"201708281428-c6b1a85417938d52f4db9e1bd5ed695b"}

post_compile escript modify app file directly, read data by file:consult and write it back by file:write_file, but this new app vsn is not taken into account by rebar.

I've found that shell hooks https://rebar3.org/docs/configuration/configuration/#shell-hooks don't update rebar state
https://github.com/2600hz/erlang-rebar3/blob/9e2e54afd74104ecb05c55e033803f41932eb940/src/rebar_hooks.erl#L17

It will be possible to add some code into rebar_hooks that updates rebar state after executing shell hooks?
Or should I rather use provider hooks, specially https://rebar3.org/docs/extending/custom_compiler_plugins for this case?

@ferd
Copy link
Collaborator

ferd commented Feb 21, 2024

You might want to just use a value of {vsn, {cmd, "some-command.escript"}} in the field in .app.src and it will be built automatically and tracked.

@nyczol
Copy link
Author

nyczol commented Feb 26, 2024

Please close this PR.
I moved vsn calc to scm_vsn plugin.
{provider_hooks, [{post, [{compile, {default, scm_vsn}}]}]}.

@nyczol nyczol closed this Feb 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants